XSTRCMP(name, nameUserAuth) compares C strings and can be bypassed with an SSH "string" that contains an embedded NUL (e.g., "ssh-userauth\0junk"). Because GetString() copies bytes and NUL-terminates, XSTRCMP will stop at the first NUL and treat it as a valid match even if the received string length differs. Use a length-checked, binary-safe comparison (e.g., validate nameSz equals the expected length and compare with XMEMCMP, or parse with GetStringRef + NameToId).

fixed

The return value of SendDisconnect() is ignored. In non-blocking I/O this can return WS_WANT_WRITE, and overwriting ret with WS_INVALID_STATE_E prevents the caller from retrying the disconnect send, potentially leaving the peer connected without receiving the disconnect. Capture/propagate the SendDisconnect() result (and only return WS_INVALID_STATE_E once the disconnect has been successfully queued/sent).

This is an error case, we do not want to preserve the connection for things like WANT_WRITE. Added comments.

XSTRCMP(name, nameUserAuth) is not binary-safe for SSH "string" fields. A malicious or non-conforming peer could send an accepted service like "ssh-userauth\0junk" and pass this check because XSTRCMP stops at the embedded NUL. Prefer a length-based comparison (check nameSz against WSTRLEN(nameUserAuth) and use XMEMCMP) or parse via GetStringRef and compare IDs via NameToId.

fixed

🔹 [Low] DoServiceAccept does not send SSH_MSG_DISCONNECT on unexpected service name
Category: Disconnect and error handling violations

The new validation in DoServiceAccept detects when the server accepts an unexpected service (not ssh-userauth) but only sets ret = WS_INVALID_STATE_E without sending SSH_MSG_DISCONNECT. By contrast, the parallel validation added in DoServiceRequest (line 6549) correctly calls SendDisconnect(ssh, WOLFSSH_DISCONNECT_SERVICE_NOT_AVAILABLE) before returning the error. Per RFC 4253 §11.1, an implementation SHOULD send SSH_MSG_DISCONNECT when it detects a protocol violation, rather than simply closing the connection. The asymmetry between the two functions suggests this was an oversight rather than an intentional design choice.

/* Requested service must be 'ssh-userauth' */
    if (ret == WS_SUCCESS) {
        const char* nameUserAuth = IdToName(ID_SERVICE_USERAUTH);
        if (nameUserAuth == NULL
            || nameSz != (word32)XSTRLEN(nameUserAuth)
            || XMEMCMP(name, nameUserAuth, nameSz) != 0) {
            WLOG(WS_LOG_DEBUG, "Accepted unexpected service: %s", name);
            ret = WS_INVALID_STATE_E;
        }
    }

Recommendation: Add a SendDisconnect call before setting the error, matching the pattern used in DoServiceRequest: (void)SendDisconnect(ssh, WOLFSSH_DISCONNECT_SERVICE_NOT_AVAILABLE); with a reason code of WOLFSSH_DISCONNECT_SERVICE_NOT_AVAILABLE or WOLFSSH_DISCONNECT_PROTOCOL_ERROR.